Skip to content

[#555] Persist language filter in localStorage#557

Merged
realproject7 merged 2 commits intomainfrom
task/555-persist-language-filter
Mar 26, 2026
Merged

[#555] Persist language filter in localStorage#557
realproject7 merged 2 commits intomainfrom
task/555-persist-language-filter

Conversation

@realproject7
Copy link
Copy Markdown
Owner

@realproject7 realproject7 commented Mar 26, 2026

Summary

Fixes #275

Saves the user's language filter selection to localStorage and restores it on revisit.

  • On language filter change, saves to localStorage.plotlink_lang
  • On page load with no lang URL param (default "all"), reads saved preference and applies it via router.replace
  • Validates saved value against LANGUAGES list before applying
  • Falls back to "all" when no saved preference or invalid value

Test plan

  • Select "Korean" language filter, navigate away, return to home — should show Korean
  • Select "All" language filter — should clear preference (shows all on return)
  • Clear localStorage manually, visit home — should show all languages
  • Direct link with ?lang=French should override saved preference

🤖 Generated with Claude Code

Save selected language to localStorage on filter change and restore
it on revisit when no lang param is in the URL. Defaults to "all"
when no saved preference exists.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel Bot commented Mar 26, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
plotlink Ignored Ignored Mar 26, 2026 8:32am

Request Review

Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Clean localStorage persistence — URL params correctly take precedence over saved preference, saved values validated against LANGUAGES list, unnecessary redirects avoided when saved=all, and try/catch handles environments without localStorage.

Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Summary

The localStorage restore logic cannot distinguish a missing lang param from an explicit ?lang=all, so it overrides valid explicit links with the saved preference.

Findings

  • [medium] FilterBar restores plotlink_lang whenever the normalized lang prop is all, but src/app/page.tsx already normalizes both a missing lang param and an explicit ?lang=all to the same lang="all" prop. That means a user who intentionally opens /?lang=all will still be redirected to their saved language, which violates the ticket scope of restoring only when there is no lang URL param.
    • File: src/components/FilterBar.tsx:58
    • Suggestion: Pass a separate boolean or raw search-param signal down from src/app/page.tsx so the restore effect only runs when the URL truly omitted lang.

Decision

Request changes until the restore logic can tell no lang param apart from an explicit ?lang=all.

Use window.location.search to detect whether lang was explicitly
provided in the URL. Only restore localStorage preference when the
param is truly absent, not when explicitly set to "all".

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE

Summary

The restore effect now checks the raw URL for a lang param before applying the saved preference, so explicit ?lang=all links are no longer overridden.

Findings

  • None.

Decision

Approve. The requested behavior is covered. Residual risk: GitHub checks were still in progress at review time.

@realproject7 realproject7 merged commit 409dd54 into main Mar 26, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug] Language 'All' filter still reverts to English — two-part fix needed

2 participants